-
Notifications
You must be signed in to change notification settings - Fork 2.1k
gcp_upi: Add exit clause to approve_csr while loop #16089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gcp_upi: Add exit clause to approve_csr while loop #16089
Conversation
|
/assign @staebler |
staebler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the commit message from
"causes the clusters to not terminate once the install is complete"
to
"causes the pod to not terminate once the install is complete"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to touch this file when the install completes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be taken care of by the wait-for install-complete command? Or is it something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something else.
release/ci-operator/templates/openshift/installer/cluster-launch-installer-upi-e2e.yaml
Line 2215 in eba0c27
| touch /tmp/install-complete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that this is copied from the template, but is there a reason why we wouldn't want to make this the while condition?
while [[ ! -f /tmp/installcomplete ]]; do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually makes better sense but it was in the templates and I thought it was there for a reason. I think this change should be fine.
|
openshift/installer#4656 has not merged yet. Waiting on that to test e2e-gcp-upi properly. |
05f68cb to
3c96b13
Compare
|
openshift/installer#4656 has merged, so the e2e-gcp-upi test should progress to the point where the changes in this PR can be tested. /test pj-rehearse |
Oops. We still need resolution for #16089 (comment). |
The approve_csr function in the e2e-gcp-upi tests does not have an exit clause for when it is done with all the approvals. This causes the function to run in the background and causes the pods to not terminate once the install is complete, causing it to timeout after 4 hours. Added a condition that was in place in the templates file here in steps in an attempt to end once all the approvals are done.
3c96b13 to
240e698
Compare
|
@rna-afk: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
With the |
|
Was looking at the pod logs and didn't look at the status. Spoke too soon I guess :) |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rna-afk, staebler The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@rna-afk: Updated the following 2 configmaps:
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The approve_csr function in the e2e-gcp-upi tests does
not have an exit clause for when it is done with all
the approvals. This causes the function to run in
the background and causes the clusters to not terminate once the
install is complete, causing it to timeout after 4 hours.
Added a condition that was in place in the templates file here in
steps in an attempt to end once all the approvals are done.